Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/api/main_test.go`:
- Around line 200-233: The test TestRunApplicationDefaultsToAllRole should
assert the shutdown path: modify the fake
StartHTTPServer/ShutdownHTTPServer/NotifySignals setup used around
runApplication so StartHTTPServer blocks on a channel until a shutdown is
triggered, then verify ShutdownHTTPServer is invoked (e.g., set a flag or send
on a channel inside ShutdownHTTPServer). Keep the same pattern used in
TestRunApplicationApiRoleStartsAndShutsDown: have NotifySignals send SIGTERM
after a short sleep, StartHTTPServer wait for a "started" signal and then block
until a "stop" signal from ShutdownHTTPServer, and assert serverStarted and that
ShutdownHTTPServer ran to ensure the full lifecycle for runApplication is
exercised when ROLE is empty.

In `@internal/core/ports/task_queue.go`:
- Around line 50-54: The Nack contract on the TaskQueue interface currently
allows implementations to "explicitly re-queue" while the original delivery is
still pending, which can create duplicate live copies; change the Nack
documentation and signature contract on Nack(ctx context.Context, queueName,
groupName, messageID string) error to explicitly forbid re-queuing or creating a
new live message ID and restrict it to only relinquishing the current delivery
(e.g., by releasing visibility/pending entry); add a new explicit method (e.g.,
Requeue(ctx context.Context, queueName, groupName, messageID string) error) that
transfers ownership/creates a new delivery and document its semantics separately
so implementations stop forking messages in Nack and instead implement the new
Requeue API for immediate re-enqueue semantics, updating any implementing types
and callers to follow the new contract.

In `@internal/drills/ha_drills_test.go`:
- Around line 359-385: The test currently races on time.Sleep when trying to
ensure the probe goroutine is the half-open probe and when waiting for the
circuit to close; replace both sleeps with explicit synchronization: add a
probeStarted channel that the goroutine running cb.Execute signals (close or
send) immediately after it becomes the probe (before blocking on probeDone) and
have the main test wait for probeStarted before issuing the looped requests, and
instead of sleeping after closing probeDone wait for either the probe to finish
(probeDone completion) or poll/wait on cb.GetState() until it equals
platform.StateClosed (or use a done channel signaled by the probe goroutine
after it returns) before asserting the closed state so the test is deterministic
and not timing-dependent.

In `@internal/drills/release_gates_test.go`:
- Around line 69-88: The test uses time.Sleep to assume the two goroutines have
acquired compute slots before calling bhCompute.Execute, which is flaky; change
the synchronization so each worker signals after it successfully enters
bhCompute.Execute (e.g., use a startedCh or another WaitGroup) and only close
blockCh / perform the rejection check after receiving two signals that the
goroutines are inside the Execute callback; update the goroutines that call
bhCompute.Execute to send on startedCh (or Done() on the second WaitGroup)
immediately after entering the Execute callback and before blocking on blockCh
so the subsequent call to bhCompute.Execute reliably observes a saturated
bulkhead.

In `@internal/platform/bulkhead_test.go`:
- Around line 104-117: The test currently uses time.Sleep to wait for the
goroutine that calls bh.Execute to finish; instead make the goroutine signal
when Execute returns and have the test wait on that signal so the final assert
is deterministic. Concretely, in TestBulkheadAvailable change the anonymous
goroutine that calls bh.Execute(...) to close a new finished channel (or call
wg.Done on a sync.WaitGroup) after bh.Execute returns, then replace
time.Sleep(10 * time.Millisecond) with a receive on that finished channel (or
wg.Wait()) so the test only proceeds to the last assert after bh.Execute has
completed.
- Around line 63-64: Replace the non-fatal assertion with a fatal one: change
assert.ErrorIs(t, err, ErrBulkheadFull) to require.ErrorIs(t, err,
ErrBulkheadFull) for the bh.Execute(...) rejection checks so the test stops
immediately on failure; apply the same replacement for the other identical
assertion later in the file (the second bh.Execute check around the other
blocking assertion).

In `@internal/platform/bulkhead.go`:
- Around line 52-71: The acquire function can grab a semaphore slot even if ctx
is already canceled; update Bulkhead.acquire to first check for pre-canceled
contexts (e.g., if ctx.Err() != nil return ErrBulkheadFull) before creating the
timer or entering any select, and similarly perform the same early-cancel check
immediately before the timed select branch so Execute will never run fn() for an
already-canceled context.

In `@internal/platform/circuit_breaker_test.go`:
- Around line 110-128: The test currently uses time.Sleep after closing
probeDone which is flaky; replace the sleep by synchronizing the probe goroutine
completion (e.g., add a probeFinished channel or sync.WaitGroup and have the
goroutine that calls cb.Execute signal it when the probe returns) so that after
close(probeDone) the test waits for the probe goroutine to finish before
asserting cb.GetState() equals StateClosed; adjust references to
probeStarted/probeDone and the anonymous goroutine that calls cb.Execute to
signal probeFinished when the probe completes to make the assertion
deterministic.

In `@internal/platform/resilient_compute_test.go`:
- Around line 209-227: The test currently uses ready/time.Sleep which only
proves goroutines started, not that StartInstance actually acquired the bulkhead
slot; modify mockCompute.wait to signal a new in-flight invocation by sending on
a channel (e.g., add entered chan struct{} to mockCompute and have wait
non-blockingly send into entered when called), initialize mock.entered =
make(chan struct{}, 2) in the test, then replace the time.Sleep/ready
synchronization: wait to receive two signals from mock.entered to ensure the
first two goroutines have acquired the semaphore before releasing others or
calling Ping, and keep checking for ErrBulkheadFull from StartInstance as
before.

In `@internal/platform/resilient_compute.go`:
- Around line 140-147: GetInstanceLogs (and similarly GetInstanceStats)
currently returns an io.ReadCloser whose underlying context is cancelled when
callProtected's deferred cancel runs; fix by creating a wrapper
cancelOnCloseReadCloser type that embeds io.ReadCloser and holds the cancel
context (cancel context created by callProtected), return an instance of that
wrapper from inside callProtected so the wrapper's Close invokes the original
ReadCloser.Close and then calls cancel(), and ensure callProtected does not
cancel the context until the wrapper's Close is invoked; update
ResilientCompute.GetInstanceLogs and ResilientCompute.GetInstanceStats to use
cancelOnCloseReadCloser to defer cancellation until the stream is closed.

In `@internal/platform/retry.go`:
- Around line 81-95: The backoffDelay function implements equal jitter (flooring
at base/2) instead of the promised full jitter; change the jitter range so the
randomized delay is uniform in [0, calculated] rather than [base/2, calculated].
In function backoffDelay use a random value from 0 up to calculated (e.g.,
rand.Int63n(int64(calculated)+1) or equivalent) and return that as the jittered
duration, keeping the earlier exponential calculation and max clamping intact;
also update the test bounds in internal/platform/retry_test.go to expect values
in [0, calculated] (or adjust assertions accordingly).

In `@internal/repositories/postgres/execution_ledger.go`:
- Around line 80-90: The "failed" branch ignores the Exec result and always
returns true, causing a race where multiple workers think they reclaimed the
job; change the code in the "failed" case to capture the Exec result (e.g., tag,
err := l.db.Exec(ctx, ... , jobKey)), check tag.RowsAffected() and only return
true when RowsAffected() > 0, otherwise return false, nil; preserve the existing
error wrapping (fmt.Errorf("execution ledger retry %s: %w", jobKey, err)) for
Exec errors so behavior matches the "running" optimistic-locking path.
- Around line 45-48: Replace the direct comparison against pgx.ErrNoRows with
errors.Is to handle wrapped errors: in the execution ledger insert error
handling (the block that checks err and references pgx.ErrNoRows and jobKey),
change the conditional from "err != nil && err != pgx.ErrNoRows" to "err != nil
&& !errors.Is(err, pgx.ErrNoRows)" so wrapped pgx.ErrNoRows values are
recognized and the function returns the same fmt.Errorf(...) only for other
errors.

In `@internal/repositories/postgres/leader_elector.go`:
- Around line 40-45: The conversion from uint64 to int64 in keyToLockID triggers
gosec G115; change the mask to use a typed constant and explicit uint64-to-int64
conversion to make intent clear and avoid overflow warnings: replace the literal
mask with uint64(math.MaxInt64) and compute int64(h.Sum64() &
uint64(math.MaxInt64)), adding an import for math; this keeps the value
non-negative and makes the cast explicit for the linter in the keyToLockID
function.

In `@internal/repositories/redis/durable_task_queue.go`:
- Around line 86-91: The XDel error is currently ignored in the Dequeue path;
change the call in the Dequeue implementation (where q.client.XDel(ctx,
queueName, msg.ID) is invoked) to capture the returned error and handle it — at
minimum log a warning or error including queueName and msg.ID (and optionally
payload) using the queue's logger (e.g., q.logger or the existing logging
facility), so transient Redis failures are visible; consider retaining behavior
(still return payload) but ensure the deletion error is logged with context so
duplicates can be diagnosed.

In `@internal/workers/cluster_worker.go`:
- Around line 65-68: The reclaimLoop goroutine is not tracked by the worker's
lifecycle; update Run to increment the worker WaitGroup before starting
reclaimLoop (call wg.Add(1)), start reclaimLoop in a small wrapper that defers
wg.Done(), and ensure Run waits for wg.Wait() after ctx.Done() so in-flight
reclaim operations using sem (the channel made with clusterMaxWorkers) finish
before returning; alternatively add wg.Done() inside the top of reclaimLoop and
call wg.Wait() in Run after cancellation.

In `@internal/workers/pipeline_worker.go`:
- Around line 21-29: The ReclaimStale handling currently acks messages when
TryAcquire returns !acquired, which drops builds while the ledger lease is still
active (pipelineStaleThreshold). Change the logic in ReclaimStale/where
TryAcquire is checked so you only call Ack when the ledger entry truly indicates
completion; for the active-lease case (state == "running" and timestamp within
pipelineStaleThreshold) do not Ack—either Nack/leave pending or extend
TryAcquire to return explicit states so you can distinguish completed vs
running; use TryAcquire, the ledger entry timestamp, and pipelineStaleThreshold
to decide (only Ack when status==completed or timestamp older than
pipelineStaleThreshold).
- Around line 132-139: The current branch treats all nil returns from
loadBuildAndPipeline as "not_found" and acks the message; change
loadBuildAndPipeline to return an explicit error (e.g., (build, pipeline, err))
or otherwise expose why it returned nil, then update the handling so only true
not-found results call ledger.MarkComplete(..., "not_found") and Ack(...); for
transient DB/backend errors (err != nil and not a not-found sentinel) do not
mark complete or ack—let the message remain/retry or requeue—and log/propagate
the error appropriately; reference loadBuildAndPipeline, build, pipeline, err,
ledger.MarkComplete, taskQueue.Ack, pipelineQueueName, pipelineGroup, msg.ID,
workerCtx, and jobKey to locate the changed code.

In `@internal/workers/provision_worker_test.go`:
- Around line 166-172: The tests currently only assert the expected side was
touched (using tt.wantAcked/tt.wantNacked and fq.acked/fq.nacked) but don’t
assert the opposite side remained untouched; update the assertions so that when
tt.wantAcked is true you also assert fq.nacked is empty, when tt.wantNacked is
true you also assert fq.acked is empty, and when neither flag is set assert both
fq.acked and fq.nacked are empty; keep the existing buf.String() log assertion
and use the same fq.acked/fq.nacked symbols to locate the checks to modify.

In `@internal/workers/provision_worker.go`:
- Around line 19-28: The reclaim window (provisionReclaimMs) is shorter than the
idempotency ledger lease (provisionStaleThreshold), which lets peers reclaim
messages and ack them while ledger entries still show "running"; update the
handling so that when a consumer sees acquired==false it does NOT
unconditionally ack the message — instead query the idempotency ledger entry for
the job and only ack if the ledger shows the job is completed or the ledger
entry is stale; otherwise leave the message unacked for the original worker to
finish. Additionally, either increase provisionReclaimMs to be >=
provisionStaleThreshold (plus a small buffer) or document the dependant timing,
and apply the same change to the other ack path referenced in the file (the code
handling acquired checks and acking in the provision worker loop).

---

Outside diff comments:
In `@internal/platform/circuit_breaker.go`:
- Around line 104-116: In Execute, a panic inside fn() can leave the
halfOpenInFlight flag set and permanently open the circuit; wrap the fn()
invocation with a defer-recover that on panic calls cb.recordFailure() (or
otherwise clears/resets halfOpenInFlight via the same internal path used by
recordFailure), then re-raises the panic so behavior is unchanged; keep the
existing error and success flows (calls to recordFailure() and recordSuccess())
intact. Target symbols: Execute, allowRequest, recordFailure, recordSuccess, and
the halfOpenInFlight flag.

---

Nitpick comments:
In `@internal/platform/resilient_storage.go`:
- Around line 151-156: The Ping method currently calls r.cb.Execute directly,
bypassing the existing bulkhead-wrapper helper; change Ping to use the same
callProtected helper as other operations so health probes go through the
concurrency limiter. Specifically, in ResilientStorage.Ping replace the
r.cb.Execute(...) block with a call to r.callProtected (or the existing
callProtected method) passing ctx and a closure that creates the 5s timeout
context and calls r.inner.Ping(ctx2), preserving the timeout and cancel
semantics and returning the inner error so Ping is subject to the same bulkhead
behavior as other methods.

In `@internal/repositories/redis/durable_task_queue.go`:
- Around line 209-219: Replace the custom containsSubstring and manual
prefix/substring checks with the standard library: import "strings", change
containsBusyGroup(s string) to return strings.HasPrefix(s, "BUSYGROUP") ||
strings.Contains(s, "BUSYGROUP"), and delete the containsSubstring function;
this removes the redundant len checks and uses the built-in, well-tested string
utilities.

In `@internal/workers/cluster_worker.go`:
- Around line 181-201: The handleProvision function currently ignores errors
from w.repo.Update (several occurrences) which can hide persistent DB failures;
change each "_ = w.repo.Update(ctx, cluster)" to capture the returned error, log
it with context (include "cluster_id" and the error) and decide per call whether
to escalate (return the error) or treat as best-effort: for status transitions
before calling provision you can log and continue, but for updates after a
provisioning failure or after marking running/failed you should log the failure
and return it to avoid leaving the cluster in a stale state; update references:
ClusterWorker.handleProvision and w.repo.Update call sites inside that function
to implement this logging/return behavior.

In `@internal/workers/leader_guard_test.go`:
- Around line 130-143: Remove the unused local variable `inner` of type
`*mockRunner` from the test: it's declared but never referenced and
`countingRunner` is passed into `NewLeaderGuard` instead; delete the `inner :=
&mockRunner{}` line and the trailing comment referencing it so the test only
constructs `countingRunner`, calls `NewLeaderGuard(elector, "test:worker",
countingRunner, newTestLogger())`, and runs `guard.Run(ctx, wg)` without an
unused variable.

In `@internal/workers/leader_guard.go`:
- Around line 69-84: The loop in the leader guard currently retries immediately
when RunAsLeader returns an error, which can spin; modify the loop around the
RunAsLeader error path (the block that checks err, ctx, g.logger, g.key) to
apply a backoff before retrying: implement a context-aware exponential backoff
with jitter (capped maximum) and use select on ctx.Done() vs time.After to abort
early, logging the backoff duration via g.logger; ensure the backoff is only
applied for transient errors (when ctx.Err() == nil) so clean shutdowns are
unaffected.